Skip to content

feat: add support to parse the -- separator for commands#10070

Open
paulbalandan wants to merge 1 commit intocodeigniter4:4.8from
paulbalandan:cli-double-hyphen
Open

feat: add support to parse the -- separator for commands#10070
paulbalandan wants to merge 1 commit intocodeigniter4:4.8from
paulbalandan:cli-double-hyphen

Conversation

@paulbalandan
Copy link
Copy Markdown
Member

Description
Adds support for POSIX -- delimiter to mean the end of options.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@github-actions github-actions bot added the 4.8 PRs that target the `4.8` branch. label Mar 25, 2026
@michalsn
Copy link
Copy Markdown
Member

Should we also update CLIRequest::parseCommand() to handle the -- separator the same way? Right now it still uses the old logic, so service('request')->getOptions() will treat --flag after -- as an option.

@paulbalandan
Copy link
Copy Markdown
Member Author

Should we also update CLIRequest::parseCommand() to handle the -- separator the same way? Right now it still uses the old logic, so service('request')->getOptions() will treat --flag after -- as an option.

Okay, I can do that. But that would also mean we would have two places doing exactly the same thing. I'm thinking to update CLIRequest and have CLI call it. What do you think?

@michalsn
Copy link
Copy Markdown
Member

Okay, I can do that. But that would also mean we would have two places doing exactly the same thing. I'm thinking to update CLIRequest and have CLI call it. What do you think?

Hmm... I agree we should avoid duplicating the parsing logic, but I'm not sure having CLI call CLIRequest is the best direction. CLI is the smaller utility, while CLIRequest is a heavier request object, so that dependency feels a bit backwards. I think it would be cleaner to extract the parsing into a small shared helper and let both CLI and CLIRequest use it.

@paulbalandan
Copy link
Copy Markdown
Member Author

Hmm... I agree we should avoid duplicating the parsing logic, but I'm not sure having CLI call CLIRequest is the best direction. CLI is the smaller utility, while CLIRequest is a heavier request object, so that dependency feels a bit backwards. I think it would be cleaner to extract the parsing into a small shared helper and let both CLI and CLIRequest use it.

I was previously thinking CLIRequest to call CLI but realized CLI is static so any changes to it will be persisted. Ok, I will go with a helper.

@paulbalandan paulbalandan force-pushed the cli-double-hyphen branch 3 times, most recently from ab4a8c2 to c9cbf70 Compare March 28, 2026 18:47
Copy link
Copy Markdown
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 PRs that target the `4.8` branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants